Skip to content

Conversation

@RalfJung
Copy link
Member

@RalfJung RalfJung commented Oct 29, 2025

Trying to figure out why #147185 regresses syn builds so much.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Oct 29, 2025
@rustbot
Copy link
Collaborator

rustbot commented Oct 29, 2025

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@RalfJung RalfJung marked this pull request as draft October 29, 2025 07:34
@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 29, 2025
@RalfJung
Copy link
Member Author

@bors try
@rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Oct 29, 2025
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 29, 2025
@rust-bors
Copy link

rust-bors bot commented Oct 29, 2025

☀️ Try build successful (CI)
Build commit: f3ba3f5 (f3ba3f55843086cd3e7e91cfccec0c492804392d, parent: 907705abea355952d84ed39cea256ed4570bdda6)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (f3ba3f5): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.4% [0.4%, 0.5%] 4
Improvements ✅
(primary)
-1.7% [-2.9%, -0.3%] 9
Improvements ✅
(secondary)
-1.0% [-1.0%, -1.0%] 1
All ❌✅ (primary) -1.7% [-2.9%, -0.3%] 9

Max RSS (memory usage)

Results (primary -2.8%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.8% [-2.8%, -2.8%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -2.8% [-2.8%, -2.8%] 1

Cycles

Results (primary 2.0%, secondary 0.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
2.0% [2.0%, 2.0%] 1
Regressions ❌
(secondary)
4.1% [2.6%, 5.6%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.8% [-4.9%, -2.7%] 2
All ❌✅ (primary) 2.0% [2.0%, 2.0%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 474.692s -> 475.969s (0.27%)
Artifact size: 390.07 MiB -> 390.07 MiB (-0.00%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Oct 29, 2025
@RalfJung
Copy link
Member Author

I have no explanation how this can make such a big difference for syn... even if syn has thousands of repr(transparent) types, this should hardly change anything as it just checks one more flag?

@RalfJung
Copy link
Member Author

Cc @rust-lang/wg-compiler-performance in case someone has any idea how this could be happening.

@RalfJung RalfJung force-pushed the perf-transparent-checks branch from 134bed6 to dd40018 Compare October 29, 2025 11:31
@RalfJung
Copy link
Member Author

@bors try
@rust-timer queue

@rust-timer

This comment has been minimized.

rust-bors bot added a commit that referenced this pull request Oct 29, 2025
@rust-bors

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 29, 2025
@rust-bors
Copy link

rust-bors bot commented Oct 29, 2025

☀️ Try build successful (CI)
Build commit: 9fcd3c8 (9fcd3c810e6e2bb555bfca0af2cc081141a7a911, parent: c6d42d774d1edfb270b8faaefacc67c213b0260b)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (9fcd3c8): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

@bors rollup=never
@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results (primary 2.1%, secondary 1.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
2.1% [2.1%, 2.1%] 1
Regressions ❌
(secondary)
3.1% [2.2%, 4.0%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.0% [-3.0%, -3.0%] 1
All ❌✅ (primary) 2.1% [2.1%, 2.1%] 1

Cycles

Results (primary 2.1%, secondary -0.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
2.1% [2.1%, 2.1%] 1
Regressions ❌
(secondary)
2.3% [2.0%, 2.6%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-3.1% [-3.4%, -2.7%] 2
All ❌✅ (primary) 2.1% [2.1%, 2.1%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 475.179s -> 475.205s (0.01%)
Artifact size: 390.26 MiB -> 390.27 MiB (0.00%)

@rustbot rustbot removed the perf-regression Performance regression. label Oct 29, 2025
@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 29, 2025
@RalfJung RalfJung force-pushed the perf-transparent-checks branch 2 times, most recently from ece2bd9 to 82b29a4 Compare October 29, 2025 18:00
@RalfJung RalfJung force-pushed the perf-transparent-checks branch from 82b29a4 to 60765e3 Compare October 29, 2025 18:01
@RalfJung
Copy link
Member Author

@bors try
@rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

rust-bors bot added a commit that referenced this pull request Oct 29, 2025
@rust-bors

This comment has been minimized.

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Oct 29, 2025
@rust-bors
Copy link

rust-bors bot commented Oct 29, 2025

💔 Test for 3a5a811 failed: CI

@Kobzol
Copy link
Member

Kobzol commented Oct 29, 2025

https://perf.rust-lang.org/detailed-query.html?commit=f3ba3f55843086cd3e7e91cfccec0c492804392d&benchmark=syn-2.0.101-check&scenario=incr-unchanged&base_commit=907705abea355952d84ed39cea256ed4570bdda6 there were some query changes, maybe that could be it? Cachegrind diff looks kinda interesting:

< -39,350,149  ???:
   -6,659,324    <&mut serde_json::ser::Serializer<&mut alloc::boxed::Box<dyn std::io::Write + core::marker::Send>> as serde_core::ser::Serializer>::serialize_str
   -4,215,955    <std::io::buffered::bufwriter::BufWriter<std::io::stdio::Stderr> as std::io::Write>::write_all
   -2,730,341    <rustc_errors::styled_buffer::StyledBuffer>::putc
   -2,580,516    <rustc_span::source_map::SourceMap>::lookup_char_pos
   -2,511,566    rustc_errors::emitter::normalize_whitespace
   -2,492,928    <rustc_errors::styled_buffer::StyledBuffer>::render
   -2,307,220    <rustc_span::SourceFile>::get_line
   -1,407,502    <rustc_errors::emitter::HumanEmitter>::draw_line
   -1,249,210    _rjem_je_arena_ralloc
     -942,782    <rustc_errors::styled_buffer::StyledBuffer>::ensure_lines
     -868,556    realloc
     -818,220    core::str::count::do_count_chars
     -803,060    <serde_json::ser::Compound<&mut alloc::boxed::Box<dyn std::io::Write + core::marker::Send>, serde_json::ser::CompactFormatter> as serde_core::ser::SerializeStruct>::serialize_field::<usize>
     -733,739    free
     -691,479    <rustc_errors::emitter::HumanEmitter>::emit_messages_default_inner::{closure#0}
     -567,521    core::fmt::write
     -544,679    rustc_incremental::persist::load::setup_dep_graph
     -535,045    <<rustc_errors::json::Diagnostic>::from_errors_diagnostic::BufWriter as std::io::Write>::write
     -510,903    malloc
     -464,106    <rustc_errors::json::DiagnosticSpan as serde_core::ser::Serialize>::serialize::<&mut serde_json::ser::Serializer<&mut alloc::boxed::Box<dyn std::io::Write + core::marker::Send>>>
     -422,919    <anstyle::style::Style>::fmt_to
     -326,780    <hashbrown::raw::RawTable<((usize, rustc_data_structures::stable_hasher::HashingControls), rustc_data_structures::fingerprint::Fingerprint)>>::reserve_rehash::<hashbrown::map::make_hasher<(usize, rustc_data_structures::stable_hasher::HashingControls), rustc_data_structures::fingerprint::Fingerprint, rustc_hash::FxBuildHasher>::{closure#0}>
      326,776    <hashbrown::raw::RawTable<((*const (), rustc_data_structures::stable_hasher::HashingControls), rustc_data_structures::fingerprint::Fingerprint)>>::reserve_rehash::<hashbrown::map::make_hasher<(*const (), rustc_data_structures::stable_hasher::HashingControls), rustc_data_structures::fingerprint::Fingerprint, rustc_hash::FxBuildHasher>::{closure#0}>
     -303,372    <std::io::default_write_fmt::Adapter<<rustc_errors::json::Diagnostic>::from_errors_diagnostic::BufWriter> as core::fmt::Write>::write_str
     -261,576    <rustc_errors::json::DiagnosticSpanLine>::from_span
     -258,560    <rustc_errors::styled_buffer::StyledBuffer>::append
     -229,844    <rustc_span::source_map::SourceMap>::span_to_lines
     -217,550    core::fmt::Formatter::pad
     -196,577    _rjem_je_arena_cache_bin_fill_small
     -186,318    rustc_span::char_width
     -165,353    <rustc_span::span_encoding::Span>::to
     -154,852    rustc_errors::emitter::emit_to_destination
     -129,414    _rjem_je_malloc_default
     -104,344    <rustc_errors::json::DiagnosticSpan>::from_span_full::<core::iter::sources::from_fn::FromFn<<rustc_span::span_encoding::Span>::macro_backtrace::{closure#0}>>
     -101,388    <anstyle::style::Style as core::fmt::Display>::fmt
      -92,904    <<rustc_errors::json::Diagnostic>::from_errors_diagnostic::BufWriter as std::io::Write>::write_fmt
      -90,530    <anstyle::color::DisplayBuffer>::write_str
      -87,516    <rustc_span::source_map::SourceMap>::span_to_snippet
      -85,554    rustc_span::str_width
      -83,035    <alloc::string::String as core::fmt::Write>::write_str
      -80,730    <serde_json::ser::Compound<&mut alloc::boxed::Box<dyn std::io::Write + core::marker::Send>, serde_json::ser::CompactFormatter> as serde_core::ser::SerializeStruct>::serialize_field::<u32>
      -79,817    <alloc::raw_vec::RawVec<rustc_expand::mbe::TokenTree>>::grow_one
       79,817    <alloc::raw_vec::RawVec<object::write::Symbol>>::grow_one
      -66,234    <rustc_errors::snippet::Style>::anstyle
      -55,212    <core::str::lossy::Utf8Chunks as core::iter::traits::iterator::Iterator>::next
      -54,930    core::ptr::drop_in_place::<rustc_errors::json::DiagnosticSpan>
      -54,173    <serde_json::ser::Compound<&mut alloc::boxed::Box<dyn std::io::Write + core::marker::Send>, serde_json::ser::CompactFormatter> as serde_core::ser::SerializeStruct>::serialize_field::<core::option::Option<alloc::string::String>>
      -53,844    rustc_middle::ty::codec::encode_with_shorthand::<rustc_middle::query::on_disk_cache::CacheEncoder, rustc_middle::ty::Ty, <rustc_middle::query::on_disk_cache::CacheEncoder as rustc_middle::ty::codec::TyEncoder>::type_shorthands>
      -53,628    <rustc_errors::emitter::HumanEmitter>::draw_line_num
      -52,752    rustc_middle::lint::explain_lint_level_source
      -50,079    <rustc_errors::emitter::HumanEmitter as rustc_errors::emitter::Emitter>::emit_diagnostic
      -49,110    _rjem_je_tcache_bin_flush_small
      -46,860    <core::iter::sources::from_fn::FromFn<<rustc_span::span_encoding::Span>::macro_backtrace::{closure#0}> as core::iter::traits::iterator::Iterator>::next
      -45,249    <rustc_errors::emitter::FileWithAnnotatedLines>::collect_annotations::add_annotation_to_file
      -44,424    <rustc_span::source_map::SourceMap>::span_until_char
      -42,623    <core::str::iter::SplitInternal<char>>::next

<  -1,618,401  ./string/../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:__memcpy_avx_unaligned_erms

<     -47,730  ./nptl/./nptl/pthread_mutex_trylock.c:pthread_mutex_trylock@@GLIBC_2.34

Maybe there are a bunch of new warnings emitted after your PR?

warning: `syn` (lib) generated 19 warnings (run `cargo fix --lib -p syn` to apply 19 suggestions)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 2.09s

vs

warning: `syn` (lib) generated 67 warnings (45 duplicates) (run `cargo fix --lib -p syn` to apply 22 suggestions)
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 2.28s

@RalfJung
Copy link
Member Author

The query changes looked entirely insignificant to me... or maybe I just don't know how to read that page.

Maybe there are a bunch of new warnings emitted after your PR?

Where did you get that output from?

We did crater this, but maybe this is an ancient version of syn that still triggers the lint. The PR also changed repr_transparent_external_private_fields to report-in-deps, after all.

@Kobzol
Copy link
Member

Kobzol commented Oct 29, 2025

I just ran cargo build with the previous/new compiler version on https://github.com/rust-lang/rustc-perf/tree/master/collector/compile-benchmarks/syn-2.0.101.

@Kobzol
Copy link
Member

Kobzol commented Oct 29, 2025

The syn version is 6 months old, for reference, so not exactly ancient.

@RalfJung
Copy link
Member Author

Ah, on the latest nightly we are getting

warning: lint `repr_transparent_external_private_fields` has been renamed to `repr_transparent_non_zst_fields`
   --> src/token.rs:146:28
    |
146 |     #[allow(unknown_lints, repr_transparent_external_private_fields)] // False positive: https://github.com/rust-lang/rust/issues/78586#issuecomment-1722680482
    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `repr_transparent_non_zst_fields`
    |
    = note: `#[warn(renamed_and_removed_lints)]` on by default

warning: lint `repr_transparent_external_private_fields` has been renamed to `repr_transparent_non_zst_fields`
   --> src/token.rs:327:36
    |
327 |               #[allow(unknown_lints, repr_transparent_external_private_fields)] // False positive: https://github.com/rust-lang/rust/issues/78586#issuecomment-1722680482
    |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `repr_transparent_non_zst_fields`
...
521 | / define_punctuation_structs! {
522 | |     "_" pub struct Underscore/1 /// wildcard patterns, inferred types, unnamed items in constants, extern crates, use declarations, and destructuring assignment
523 | | }
    | |_- in this macro invocation
    |
    = note: this warning originates in the macro `define_punctuation_structs` (in Nightly builds, run with -Z macro-backtrace for more info)

warning: lint `repr_transparent_external_private_fields` has been renamed to `repr_transparent_non_zst_fields`
   --> src/token.rs:327:36
    |
327 |               #[allow(unknown_lints, repr_transparent_external_private_fields)] // False positive: https://github.com/rust-lang/rust/issues/78586#issuecomment-1722680482
    |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use the new name: `repr_transparent_non_zst_fields`

And I guess what we are seeing is the effort of rendering those lints.

Good catch, thanks!

@RalfJung RalfJung closed this Oct 29, 2025
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 29, 2025
@Kobzol
Copy link
Member

Kobzol commented Oct 29, 2025

Yeah, the diagnostics emitting code is ludicrously slow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-perf Status: Waiting on a perf run to be completed. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants